-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enriched metadata with date, game_week and game_id #340
Enriched metadata with date, game_week and game_id #340
Conversation
Thanks a lot for this PR! Some questions/feedback:
happy to help where needed. |
@koenvo Thanks as well for the quick feedback! The game_week is information on the number of games played by a team, within a league, including the current one. For instance, the first gameweek in Premier League will start on the 16th of August at 7:30pm. So it is mainly an integer from 1 to 38 in the PL, 1 to 34 in the Ligue 1 (since there are only 18 teams now), and 30 in the Jupiler Pro League. Moreover, if the game is happening during a cup or during a play-off, then the information will be "8th Finals". |
e07822c
to
7e7a25f
Compare
Please let me know if anything is still unclear |
7e7a25f
to
ebb17ed
Compare
It would probably be helpful to keep track of all the providers / deserializers we are supporting this behavior for. This PR supports:
Tracking Providers
Am I missing anything? |
@UnravelSports This is correct yes. |
ebb17ed
to
3881606
Compare
@UnravelSports I just pushed the implementation for Events/sportec (the test is similar to the implementation for Tracking/sportec). So the updated coverage is : This PR supports: Event Providers
Tracking Providers
|
I've been looking at the existing test files to see if we can learn anything from them for the currently unsupported providers. But, I'm not 100% sure if the values that I'm listing here are the correct ones. I personally don't have access to any of the required files, except for SkillCorner and StatsBomb, but I think we can find most of them in the test files. Event Providers
Tracking Providers
|
82fa271
to
8eaeb0b
Compare
Thanks @UnravelSports , this is really helpful! Based on your investigation, I was able to implement enrichment for :
Metrica's "FileDate" seems to represent the last time the file has been updated. Which is not the information we are looking for. The updated coverage is then : Event Providers
Tracking Providers
|
Awesome additions @SportsDynamicsDS! Okay, so for StatsBomb the matchIds are not provided in the event files, rather they are provided in the match files. Unfortunately, I don't think it's feasible to pass the matches file to @koenvo @probberechts @JanVanHaaren do you think it would be a solution to add
|
We also checked for Hawkeye, and the only available information is the game_id. @probberechts talking about Hawkeye, we were able to run successfully your deserializer (Issue 324) on a few Champions League games, and it worked as expected 👍 |
Yes, maybe best to just pass |
Yes, that solution sounds good to me! I can think of use cases where a user might want to include additional meta data such as the identity of the coach. |
8eaeb0b
to
6ac5e4b
Compare
Thanks for the feedback guys! Moreover, as @JanVanHaaren suggested, "home_coach" and "away_coach" are now parameters of the Metadata class. For WyScout and SkillCorner, the information is automatically extracted. So the updated coverage is : This PR supports: Event Providers
Tracking Providers
|
6ac5e4b
to
d81ae7d
Compare
Any update on this PR ? Thanks 🙏 |
Hello everyone,
Happy to contribute for the first time to the package!
Working with the Metadata class, we noticed there is a few missing pieces of information that could be useful :
In this PR :
Feel free to contact us if anything is unclear or need to be updated.
Thanks,
The SportsDynamics team
PS : I started with a PR instead of creating an issue, but I have checked if no issue mentioned this topic.